Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to react-router@7, eslint@9, version bump the rest #101

Merged
merged 18 commits into from
Dec 27, 2024
Merged

Conversation

vcarl
Copy link
Member

@vcarl vcarl commented Dec 20, 2024

Goal here is to get fully up-to-date with modern Remix/React Router. I've gotten this working ~ correctly (as best I can tell right now) with hot reloading for server/Remix/client (which are different things! the Express server and Remix SSR are built separately, it's a PITA).

This is a draft PR while I continue working on moving to React Router v7 (from Remix v2).

@vcarl vcarl marked this pull request as ready for review December 24, 2024 08:19
@vcarl
Copy link
Member Author

vcarl commented Dec 24, 2024

Aight I got there 👍 Full upgrade from Remix v1 -> v2 -> React Router v7 (which ended up breaking linting, so that's now on eslint v9 too)

@vcarl vcarl changed the title Major dependencies upgrade Upgrade to react-router@7, eslint@9, version bump the rest Dec 24, 2024
@vcarl
Copy link
Member Author

vcarl commented Dec 24, 2024

Whoops bot doesn't actually run, just the webapp

@vcarl vcarl force-pushed the vc-deps-update branch 2 times, most recently from 09bd881 to d686032 Compare December 26, 2024 23:54
@vcarl
Copy link
Member Author

vcarl commented Dec 26, 2024

Rebased to collapse some "oops"-tier commits. This is actually like 3x migrations in 1 PR which makes me a bit nervous, but I've validated about as much as I can locally. Caught a few snafus, fingers crossed there aren't any weirder ones — this does a lot to change the entrypoints and adds a new prod/dev check in order to enable hot reload functionality, so it's pretty high risk. Will be monitoring and validate the deploy once it lands

@vcarl vcarl merged commit 89f7ae2 into main Dec 27, 2024
5 checks passed
@vcarl vcarl deleted the vc-deps-update branch December 27, 2024 00:05
vcarl added a commit that referenced this pull request Dec 27, 2024
@vcarl
Copy link
Member Author

vcarl commented Dec 28, 2024

Welp this doesn't actually do a correct production build, it's missing the outside server.js function that actually starts the bot + http server. This is a pretty fragile and complicated setup unfortunately to get working:

  • react-router has a cli that does a build. The "framework mode" is actually a pretty complicated mix of bundler plugins, typegen, build CLI, dev server… it turns it guts inside out so at least I'm not monkeypatching anything, but it means I have to understand how it's glued together.
  • The template here basically punts on the issue of integrating into the build by introducing an entirely-separate server.js script that doesn't go through any other part of the build process. This does not work here, because I have more than just an express server, and the server script relies on TS configuration to appropriately run.
  • This runs in dev currently by executing tsx app/index.ts, but tsx doesn't work for compiling a production build. tsc -b either duplicates work done by react-router build, or produces an incomplete/non-functional output. Because the folder structure isn't preserved between dev/prod, it seems confusing to set up a single script that exists in 2 different contexts.

I think I can move the code around such that there's a dev/prod script that starts up the server appropriately, but it's diverging a good bit from the template. Since this asks for a plain-JS startup script, I'm going to see if I can get that working with minimal fuss, without making it too difficult to understand and change.

vcarl added a commit that referenced this pull request Dec 28, 2024
)

Had to revert #101 because it wasn't starting up correctly; the built
artifact did not include the actual express startup code. This PR copies
in the startup script via Docker because of how the React Router v7
"framework mode" build tool functions. #101 has the original motivation
for the change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant